Skip to content

Conversation

@ga-devfront
Copy link

Fix phpstan/phpstan#13353
This is the fix for the issue above. If you see any changes to make, please let me know; this is my first contribution to the project.

@ga-devfront ga-devfront force-pushed the fix/variable-undefined branch from fd4fefe to 8b91913 Compare August 7, 2025 12:18
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at this problem more thoroughly. There's IssetCheck (

if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
) used by several rules where only Variable with string name property is being handled, but the code might find some improvements to the logic useful.

@ga-devfront ga-devfront force-pushed the fix/variable-undefined branch from db07673 to 7733eb3 Compare September 1, 2025 15:54
@ga-devfront
Copy link
Author

Please look at this problem more thoroughly. There's IssetCheck (

if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {

) used by several rules where only Variable with string name property is being handled, but the code might find some improvements to the logic useful.

It was my first idea to fix the problem in the IssetCheck, however, the error being returned actually comes from DefinedVariableRule. Being new to the project, I don't yet understand how it all works. However, if you could tell me a little more about what you'd like to see added, there's no problem; I'm open to any suggestions.
I've added the tests as requested.

@ondrejmirtes
Copy link
Member

IssetCheck is used by IssetRule and others. Look at IssetRuleTest what it detects.

You should be able to produce some tests with $$ to see what's currently wrong about the behaviour.

@ga-devfront
Copy link
Author

IssetCheck is used by IssetRule and others. Look at IssetRuleTest what it detects.

You should be able to produce some tests with $$ to see what's currently wrong about the behaviour.

As you can see, I added the tests in IssetRuleTest.php, but nothing is thrown.
Maybe I missed something, feel free to let me know.

@VincentLanglet
Copy link
Contributor

The branch has conflict @ga-devfront ; are you interested in rebasing/finishing the PR ?

@ga-devfront ga-devfront force-pushed the fix/variable-undefined branch from 4c8527b to 8270c51 Compare October 20, 2025 07:19
@ga-devfront
Copy link
Author

The branch has conflict @ga-devfront ; are you interested in rebasing/finishing the PR ?

Rebase done. But don't know what to do more for finishing it.

@ga-devfront
Copy link
Author

@ondrejmirtes up please ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[variable.undefined] Variable $wow might not be defined.

3 participants